Lots of bug fixes relating to failed builds and recovery from them #550
Conversation
| await buildStep.writeAsString( | ||
| buildStep.inputId.changeExtension(jsModuleErrorsExtension), '$e'); | ||
| log.severe('', e); |
There was a problem hiding this comment.
This is just logging the error - it feels a bit weird for sure but there is already additional context provided by the loggers name (it is named based on the current builder and primary input).
The logger handles consistently formatting the error already as well - so that ends up being nicer than passing the error as the message.
There was a problem hiding this comment.
Ya - we don't have a true stack trace here. This error is originating from a worker, and contains the entire message that the worker sent back - which may or may not contain a useful stack trace already.
Ya, this is effectively being tested with the ddc error recovery e2e test, but some unit tests would be good as well, I can work on that. |
|
Travis is having issues today, going to run with a local |
SyntheticAssetNode. These are created if a build step callscanReadfor an asset that doesn't exist. Those files still need to be tracked so if they are added later on that build step gets invalidated.scopeLogtoscopeLogAsync, since its always used in an async context, and we now run in an error handling zone that logs errors tologger.severe. Any call toscopeLogAsyncis now guaranteed to eventually complete with exactly one error or value, and will not leak unhandled async errors.runBuilderto pass in a logger whose name is$builder on $input, so you get more context for errors.computeTransitiveModulesto check that modules exist before trying to read them, and warn if they don't. Eventually we probably want this to fail the build as well, but only on some equivalent of "--mode=release". For dev time we probably don't want any builds to fail so the app can serve errors in a friendly fashion.updateAndInvalidatewhere it would delete nodes that shouldn't be deleted.Fixes #548.